Skip to content

Remove Sketchbook concept, introduce User data folder #516

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 19, 2019

Conversation

masci
Copy link
Contributor

@masci masci commented Dec 13, 2019

Rationale

After some QA iterations we decided to fade out the Sketchbook concept from the CLI. Please note the Sketchbook will continue existing on the IDEs, just it will be implemented at application level, not at the CLI level.

arduino-cli sketch new and other commands already stopped relying on the Sketchbook, assuming the current path will be used when no destination path is provided but the "Sketchbook" keyword continued to be present in some places in the CLI, most notably the configuration file, causing confusion among users.

Implications

The Sketchbook isn't only used to collect sketch files, it also contains custom libraries and custom hardware under the paths <sketchbook_path>/libraries and <sketchbook_path>/hardware respectively - this functionality will be preserved by introducing a new concept at the CLI level: the User folder.

This PR removes the Sketchbook dir from the settings and introduces a new concept, the "User" folder. User folder will have the same layout as the existing "Data" folder and will contain "custom stuff" like libraries and hardware. Users can keep using it to store sketch files, just the CLI won't care nor assume anything about those.

Backward compatibility

In order to keep backward compatibility, the environment var ARDUINO_SKETCHBOOK_DIR will keep existing and point to the User folder. The old sketchbook_path value will be removed from the settings instead and user will use this new configuration value in arduino-cli.yaml:

directories:
  data: /home/massi/.arduino15
  user: /home/massi/Arduino

gRPC interface

At this moment the gRPC won't be touched and will keep showing references to the Sketchbook. That part will be refactored to improve the way gRPC clients can pass configuration values without having to rely on a config file.

@masci masci force-pushed the massi/no-sketchbook branch from 248c3f9 to 2ccd77d Compare December 13, 2019 14:25
@masci masci added status: on hold Do not proceed at this time and removed status: on hold Do not proceed at this time labels Dec 13, 2019
@masci masci added this to the 0.7.0 milestone Dec 18, 2019
@cmaglie
Copy link
Member

cmaglie commented Dec 19, 2019

Everything looks good to me except:

The old sketchbook_path value will be removed from the settings instead and user will use this new configuration value in arduino-cli.yaml

I'd keep this option so users that have an old config file will not suffer breaking changes.
I'm ok to remove it from config dump so new configurations will use the new values.

@rsora
Copy link
Contributor

rsora commented Dec 19, 2019

@cmaglie keeping the sketchbook_path options unfortunately will continue to create issues as #503 in which users are not able to figure out why the sketchbook concept does not work properly in the CLI.

I suggest to go ahead and merge, we will explain precisely in the changelog the effects of this change and how to migrate the arduino-cli.yaml to be used under the next release.

@rsora rsora self-requested a review December 19, 2019 14:49
@masci
Copy link
Contributor Author

masci commented Dec 19, 2019

@pirropirro FYI we're merging this

@masci masci merged commit f11f161 into master Dec 19, 2019
@masci masci deleted the massi/no-sketchbook branch December 19, 2019 15:30
rsora pushed a commit that referenced this pull request Dec 19, 2019
* rename Sketchbook folder to User

* missing join on path
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants